Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(node): Add socketIoIntegration to node #13578

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

Zen-cronic
Copy link
Contributor

@Zen-cronic Zen-cronic commented Sep 3, 2024

Resolves #13310

  • If you've added code that should be tested, please add tests.
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

@Zen-cronic
Copy link
Contributor Author

Zen-cronic commented Sep 5, 2024

Tests to be added. WIP

@Zen-cronic Zen-cronic marked this pull request as ready for review September 14, 2024 01:14

createRunner(__dirname, 'scenario.js')
.expect({ transaction: SERVER_TRANSACTION })
.expect({ transaction: CLIENT_TRANSACTION })
Copy link
Contributor Author

@Zen-cronic Zen-cronic Sep 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why the server socket and client socket are not part of the same transaction. In scenario.js, the server socket emits an event to the client, which triggers the client to send back to the server.

Update: the transactions are also not part of the same trace (different trace_id) - verified by debugging.

Add otel integration and e2e tests.

Signed-off-by: Kaung Zin Hein <[email protected]>
@Zen-cronic Zen-cronic reopened this Sep 14, 2024
onHook(span) {
addOriginToSpan(span, 'auto.socket.otel.consumer');
},
traceReserved: true,
Copy link
Contributor Author

@Zen-cronic Zen-cronic Sep 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should expose options such as traceReserved, emitIgnoreEventList, and onIgnoreEventList for users to configure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add socketIoIntegration to Node
2 participants